Skip to content

Consume the editor from paranext-core's staged copy - #119

Merged
tjcouch-sil merged 4 commits into
mainfrom
pt-4500-scripture-editors-move
Sep 14, 2026
Merged

tjcouch-sil merged 4 commits into
mainfrom
pt-4500-scripture-editors-move

Conversation

@tjcouch-sil

@tjcouch-sil tjcouch-sil commented Aug 31, 2026 •

Copy link
Copy Markdown
Member

src/paratext-bible-text-collection pinned @eten-tech-foundation/platform-editor@~0.8.13 and scripture-utilities@~0.1.6 from the npm registry. Those packages are no longer published from the repo we develop them in.

Part of PT-4500.

Why this repo needs a change at all

scripture-editors now lives at paranext/scripture-editors, and paranext-core stages it into dev-packages/staging/ rather than installing it from npm. Left as registry pins, this package would stay frozen at whatever ETEN last published — it was already a version behind core (~0.8.13 vs ~0.8.15), and the source has since moved to 0.8.16.

The change

Both deps now point at paranext-core's staged copy:

"@eten-tech-foundation/platform-editor": "file:../../../paranext-core/dev-packages/staging/platform-editor"
"@eten-tech-foundation/scripture-utilities": "file:../../../paranext-core/dev-packages/staging/scripture-utilities"

Same shape and same relative depth as the platform-bible-react, platform-bible-utils, and papi-dts links this package already uses, so it needs nothing new locally: a sibling paranext-core checkout, which is already required.

CI needed a step, or this would have shipped broken

dev-packages/staging/ is gitignored in core and produced by core's preinstall. actions/checkout does not create it, and both workflows install this repo before core and then install core with npm ci --ignore-scripts — which skips that very preinstall.

npm does not report a missing file: target. It creates a broken symlink and exits 0:

$ cat package.json
{ "name": "t", "dependencies": { "faux": "file:./does-not-exist" } }
$ npm install
added 1 package in 195ms          # exit 0
$ ls -l node_modules/faux
faux -> ../does-not-exist         # broken

So CI would have installed "successfully" and failed later with an unresolved module, far from the cause. Both workflows now stage core's dev packages first:

- name: Stage paranext-core's dev packages
  working-directory: paranext-core
  run: npm run stage-dev-packages

It needs only git and node — the script has no npm dependencies, which is exactly why core uses it as its own preinstall.

The lockfile, and where the closure comes from

package-lock.json is refreshed here, regenerated against a paranext-core carrying the staging change. Both entries are now link: true at the staged versions (platform-editor 0.8.16, scripture-utilities 0.1.6).

It also drops ~135 entries. That is expected: npm resolves a file: target's dependency closure only when the target is inside the consuming project's root. Core is a sibling, so its tree is no longer restated here. Those dependencies resolve at runtime through the symlink's realpath out of core's own node_modules — verified for lexical and @floating-ui/dom — which is precisely how this repo's existing file:../paranext-core/lib/... dependencies have always worked.

Merge order

Unblocked. paranext-core#2745 and scripture-editors#1 have both merged, and platform-yalc has been
moved onto scripture-editors' new main, so the pinned revision carries its committed dist/ and
the staging step copies rather than building — which is the only shape this runner supports, since
there is no pnpm here.

Verification

With an updated paranext-core checked out beside this repo:

  • npm install — lockfile regenerated and committed, both links resolve
  • npm run lint — clean
  • npm run build — both bundles compile (webView and main), which is the path publish.yml exercises via npm run package

Related

paratext-assistant#130 makes the equivalent change. An earlier version of this description claimed this repo was the only consumer outside paranext-core; that was wrong — paratext-assistant also depends on scripture-utilities, and it needed the same workflow step.

🤖 Generated with Claude Code

https://claude.ai/code/session_017jvJ2eYTfxJaBkKoQbVeiW


This change is Reviewable

This package pinned @eten-tech-foundation/platform-editor ~0.8.13 and
scripture-utilities ~0.1.6 from the npm registry. Those packages are no longer published
from the repository we develop them in: scripture-editors now lives at
paranext/scripture-editors, and paranext-core builds it from source and stages the result
into dev-packages/staging/ rather than installing it from npm. Left as registry pins,
this package would stay frozen at whatever ETEN last published.

Points both at paranext-core's staged copy, matching how this repo already consumes
platform-bible-react, platform-bible-utils, and papi-dts from a sibling paranext-core
checkout.

package-lock.json is deliberately not updated here — regenerating it requires a
paranext-core that already carries the staging change, so it should be refreshed once
that has merged.
@tjcouch-sil

Copy link
Copy Markdown
Member Author

Marking this a draft: CI is red and will stay red until paranext-core#2745 merges.

npm ci fails with Missing: @eten-tech-foundation/platform-editor@ from lock file — the lockfile is deliberately not updated here, because regenerating it needs a paranext-core that already carries the staging change. Doing it now would bake a resolution against an unmerged branch into this repo.

To finish once core has merged:

# with an up-to-date paranext-core checked out as a sibling
npm install
git commit -am 'build: refresh lockfile for the staged editor'

That will also clear the stale nested platform-editor@0.8.14 entries already in the lockfile today.

The editor `file:` specifiers point into paranext-core's
`dev-packages/staging/`, which is gitignored there and produced by core's
`preinstall`. Checking core out does not create it, and both workflows
install this repo before core and then install core with
`--ignore-scripts`, which skips that `preinstall`. npm links a missing
`file:` target and exits 0, so CI would have installed against a broken
symlink and failed later as an unresolved module. Stage core's dev
packages first instead; the script needs only git and node, which is why
core can run it as its own `preinstall`.

The lockfile drops the editor's transitive closure because npm resolves a
`file:` target's dependencies only when the target is inside the consuming
project's root, and core is a sibling. Those dependencies resolve through
the symlink's realpath from core's own `node_modules`, as this repo's
existing `file:../paranext-core/lib/...` dependencies already do.

Verified with an updated core beside it: lint clean, and both webpack
bundles compile.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017jvJ2eYTfxJaBkKoQbVeiW
@rolfheij-sil

Copy link
Copy Markdown
Contributor

🤖 Claude: Reviewed as part of the eight-repo set; this one and paratext-assistant#130 are the two with hand-written bodies, and both are accurate. Live-tested against a core checkout carrying paranext-core#2745: npm ci (~15 s), lint and build pass; @eten-tech-foundation/platform-editor and scripture-utilities resolve into paranext-core/dev-packages/staging/…, and from inside the resolved PBR lexical, @floating-ui/dom, @xmldom/xmldom resolve into core's node_modules — no registry copies under this repo; the lockfile is clean before and after a second plain npm install. Your "npm exits 0 with a dangling symlink for a missing file: target" claim reproduces.

  • The Aug 31 comment ("the lockfile is deliberately not updated here") is superseded — the tip refreshes it (link: true entries, the nested platform-editor@0.8.14 block and ~25 @lexical/* sub-entries gone). Worth a one-line follow-up so a reviewer doesn't read the old comment as current.
  • Small robustness suggestion for the new CI step (same as on the other seven): run: npm run stage-dev-packages instead of run: node .erb/scripts/stage-dev-packages.ts — no dependency on core's internal file path.
  • Aside, pre-existing and not for this PR: this repo's root has react@18.3.1 while core (which PBR and the webviews actually run against) has react@19.2.4; runtime is fine because react/react-dom are webpack externals, but this repo's typecheck/lint never see the React 19 types it runs under.

LGTM after paranext-core#2745 and scripture-editors#1 + the platform-yalc move.

The staging step ran `node .erb/scripts/stage-dev-packages.ts`, so eight repos
depended on the location of a file inside paranext-core; renaming or moving it
would have broken all of them at once, silently, since nothing here references
it in a way a search from core would find.

`npm run stage-dev-packages` is core's own entry point for the same thing. It
needs nothing installed, so it still works before core's `npm ci`, and it
survives the file moving. Core's script also carries
`--experimental-strip-types`, which is what lets a repo whose Volta pin predates
Node 22.18 run it at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
@tjcouch-sil

Copy link
Copy Markdown
Member Author

Thanks — all three points taken.

npm run stage-dev-packages — done, here and in all eight repos, so the step is byte-identical across the 16 workflow occurrences again. Core's script also carries --experimental-strip-types, which matters for the one repo pinned below Node 22.18 and costs nothing on newer Node.

PR body corrected — the pnpm sentence was left over from the first commit and the second one removed the pnpm setup, so it described a workflow that no longer exists.

Merge order — the body now carries it, including that this stays red after core#2745 merges until scripture-editors#1 has merged and platform-yalc has moved past it so the pinned revision carries a committed dist/. Without that, staging falls back to a build this runner cannot do.

Also on the branch since your review: the branch is rebased onto current main.

The README follow-up (that a sibling paranext-core must be installed, not just cloned) is a fair point and I've left it — it belongs in a README pass rather than in a workflow PR, and it applies to all eight repos equally.

paranext-core's postinstall checks that the staged editor packages match
core's package-lock.json. This repo installs core with
`npm ci --ignore-scripts` - required, because core's postinstall builds an
Electron DLL this repo has no use for - so that check never ran here, and a
build could resolve the editor from whatever the pinned branch happened to
hold at that moment.

`npm run verify:dev-packages`, added in paranext-core#2745, is that check on
its own. It reads the tree and reports; it never installs or repairs anything.

Also refreshes package-lock.json against core's current tree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
@tjcouch-sil
tjcouch-sil marked this pull request as ready for review September 11, 2026 23:57
@tjcouch-sil
tjcouch-sil merged commit 82def8f into main Sep 14, 2026
5 of 6 checks passed
@tjcouch-sil
tjcouch-sil deleted the pt-4500-scripture-editors-move branch September 14, 2026 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants